Minimal work to make System.Diagnostics.Activity usable by the dotnet CLI application and codebase #49749
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR makes System.Diagnostics.Activity usable in the .NET CLI by creating a central ActivitySource, freezing common telemetry data for zero-cost scenarios, and updating environment-variable APIs for nullability.
- Introduce
Activities.Sourceand emitActivityEvents inTelemetry.TrackEvent - Convert common properties/measurements to
FrozenDictionaryand update nullability inTelemetry - Update forwarding-app environment‐variable methods to accept nullable values and add the
DiagnosticSourcepackage
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Cli/dotnet/Telemetry/TelemetryCommonProperties.cs | Return FrozenDictionary<string, string> instead of Dictionary for common props |
| src/Cli/dotnet/Telemetry/Telemetry.cs | Hook into Activity.Current.AddEvent, implement CreateActivityEvent/MakeTags, use frozen collections and nullables |
| src/Cli/Microsoft.DotNet.Cli.Utils/Activities.cs | Add Activities.Source static ActivitySource for the CLI |
| src/Cli/dotnet/Commands/MSBuild/MSBuildForwardingApp.cs | Accept string? for environment‐variable values |
| src/Cli/Microsoft.DotNet.Cli.Utils/MSBuildForwardingAppWithoutLogging.cs | Change internal dictionaries to store string? values for env vars |
| src/Cli/Microsoft.DotNet.Cli.Utils/ForwardingAppImplementation.cs | Update WithEnvironmentVariable to accept string? |
| src/Cli/Microsoft.DotNet.Cli.Utils/Microsoft.DotNet.Cli.Utils.csproj | Add System.Diagnostics.DiagnosticSource package reference |
| eng/Versions.props | Bump SystemDiagnosticsDiagnosticSourcePackageVersion |
| Directory.Packages.props | Pin System.Diagnostics.DiagnosticSource version |
Comments suppressed due to low confidence (2)
src/Cli/dotnet/Telemetry/Telemetry.cs:184
- [nitpick] The property key "event id" uses a space and lowercase naming. Consider using a consistent identifier like "EventId" or "eventId" without spaces for clarity.
eventProperties.Add("event id", Guid.NewGuid().ToString());
src/Cli/Microsoft.DotNet.Cli.Utils/Activities.cs:19
- This new public API for
Activities.Sourceand its integration into telemetry warrants added tests to verify that Activities are created correctly and events propagate as expected. Please add tests and use the Skip parameter on the Fact attribute to reference the related SDK issue when necessary.
public static ActivitySource Source { get; } = new("dotnet-cli", Product.Version);
This was referenced Jul 12, 2025
baronfel
force-pushed
the
add-activities-to-cli
branch
from
f0d6f8a to
c173b9e
Compare
Open
3 tasks
KalleOlaviNiemitalo
suggested changes
Jul 16, 2025
baronfel
force-pushed
the
add-activities-to-cli
branch
from
c173b9e to
62f9dcf
Compare
MiYanni
approved these changes
Jul 21, 2025
|
Thanks! I'll do that ASAP |
5 tasks
… CLI application and codebase
don't return frozen dictionaries on modifiable call-sites. combine code for combining dictionaries
baronfel
force-pushed
the
add-activities-to-cli
branch
from
a9e1761 to
99e3951
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm breaking out the monolithic set of changes from #49409 into a more digestible series of changesets.
This first one adds the ability to use System.Diagnostic.Activity at all in our codebase - creating an ActivitySource for us to use, and hooking the existing telemetry publishing up to it in a zero-cost way if no Activity Listeners are configured (which is the case today).
This is part of #49668.